From 7fbd971f2d8e803d689c7635d1564a5532d46618 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Wed, 20 Feb 2019 14:41:55 -0500
Subject: [PATCH] Database: Have buildStringCast() actually cast for MySQL,
 MSSQL

While these databases allow implicit casts (at least from int to
string), in MySQL at least the implicit cast can prevent appropriate
index usage (see T216183). So let's have buildStringCast() actually do
the cast.

This also changes the base implementation in Wikimedia\Rdmbs\Database to
explicitly cast. Any other subclasses should check whether this new
version works for them.

Bug: T216247
Change-Id: I98c67c857b35de3191d47ab28810d8eb21ddbbc8
---
 RELEASE-NOTES-1.33                                 | 3 +++
 includes/libs/rdbms/database/Database.php          | 4 +++-
 includes/libs/rdbms/database/DatabaseMssql.php     | 5 +++++
 includes/libs/rdbms/database/DatabaseMysqlBase.php | 4 ++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index 957384b395..56d06a08a3 100644
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -309,6 +309,9 @@ because of Phabricator reports.
 === Other changes in 1.33 ===
 * (T201747) Html::openElement() warns if given an element name with a space
   in it.
+* The implementation of buildStringCast() in Wikimedia\Rdbms\Database has
+  changed to explicitly cast. Subclasses relying on the base-class
+  implementation should check whether they need to override it now.
 
 == Compatibility ==
 MediaWiki 1.33 requires PHP 7.0.13 or later. Although HHVM 3.18.5 or later is
diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 974c7df46e..b3597df019 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2278,7 +2278,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	}
 
 	public function buildStringCast( $field ) {
-		return $field;
+		// In theory this should work for any standards-compliant
+		// SQL implementation, although it may not be the best way to do it.
+		return "CAST( $field AS CHARACTER )";
 	}
 
 	public function buildIntegerCast( $field ) {
diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index 93bb5d3bfd..a6027e6bea 100644
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -1398,6 +1398,11 @@ class DatabaseMssql extends Database {
 
 		return $old;
 	}
+
+	public function buildStringCast( $field ) {
+		return "CAST( $field AS NVARCHAR )";
+	}
+
 }
 
 /**
diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 186c89f3d6..62110ef45a 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -1548,6 +1548,10 @@ abstract class DatabaseMysqlBase extends Database {
 			!preg_match( '/^SELECT\s+(GET|RELEASE|IS_FREE)_LOCK\(/', $sql );
 	}
 
+	public function buildStringCast( $field ) {
+		return "CAST( $field AS BINARY )";
+	}
+
 	/**
 	 * @param string $field Field or column to cast
 	 * @return string
-- 
2.20.1